Skip to content

MNT: install contextily with the other optional requirements - #1179

Open
ting-hong-shieh wants to merge 3 commits into
RocketPy-Team:developfrom
ting-hong-shieh:mnt/optional-requirements-contextily
Open

MNT: install contextily with the other optional requirements#1179
ting-hong-shieh wants to merge 3 commits into
RocketPy-Team:developfrom
ting-hong-shieh:mnt/optional-requirements-contextily

Conversation

@ting-hong-shieh

Copy link
Copy Markdown

The gap

contextily is declared in the monte-carlo extra in pyproject.toml:

monte-carlo = [
    "imageio",
    "multiprocess>=0.70",
    "statsmodels",
    "prettytable",
    "contextily>=1.0.0; python_version < '3.14'",
]

but not in requirements-optional.txt, and that is what the documented setup installs:

install:
	$(PYTHON) -m pip install --upgrade pip
	pip install -r requirements.txt
	pip install -r requirements-optional.txt
	pip install -r requirements-tests.txt
	pip install -e .

tests/unit/simulation/test_monte_carlo_plots_background.py opens with pytest.importorskip("contextily"), so the whole file is skipped.

What it costs

Measured at 4263fa95d7fe6f63d9593f01e4ff7a088369e195, running the five steps of test_pytest.yaml in order with --cov-append:

Without contextily With it
plots/monte_carlo_plots.py 89 missed 36 missed
Project, non-slow 14,771 covered, 84.2757% 14,824 covered, 84.5781%

Codecov reports 84.57% for that commit, and 14,824 hits against 2,703 misses. The second row is the one that matches; the first is what a contributor following the Makefile sees.

Tests installs .[all], which does resolve the extra on Python 3.10, so CI is unaffected either way. What this fixes is a local run quietly disagreeing with CI by 53 statements, with a skip reason buried in -rs output as the only clue.

The change

One line, the same specifier as pyproject.toml, marker included:

contextily>=1.0.0; python_version < '3.14'

Verification

Before, in an environment built from the requirements files:

$ pytest tests/unit/simulation/test_monte_carlo_plots_background.py -q
SKIPPED [1] tests/unit/simulation/test_monte_carlo_plots_background.py:17: This test requires contextily to be installed
1 skipped in 0.03s

After installing it:

$ pytest tests/unit/simulation/test_monte_carlo_plots_background.py -q
18 passed, 16 warnings in 37.14s

Those 18 do reach the network — they call contextily.bounds2img against real tile providers rather than stubbing it. That is existing behavior and not changed here, but it is worth knowing that this makes the default local suite contact Esri, OpenStreetMap and CartoDB where it previously did not. Happy to close this instead if the team would rather those tests were stubbed first, or would rather keep them opt-in.

Found while re-measuring the baseline for #709; it is why the figures in my first comment there were 53 statements low.

contextily is declared in the monte-carlo extra in pyproject.toml but not
in requirements-optional.txt, and the Makefile's install target reads the
requirements files. test_monte_carlo_plots_background.py opens with
pytest.importorskip("contextily"), so anyone who sets up with `make
install` skips that file: 18 tests, and 53 statements that Codecov counts
as covered.

The workflow installs .[all], so CI already has it and is unaffected. What
this fixes is the local suite silently disagreeing with CI, with a skip
reason as the only clue.

Same specifier as pyproject.toml, including the 3.14 marker.
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.96%. Comparing base (4263fa9) to head (e054e8a).
⚠️ Report is 2 commits behind head on develop.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1179      +/-   ##
===========================================
+ Coverage    84.57%   89.96%   +5.39%     
===========================================
  Files          131      131              
  Lines        17527    17527              
===========================================
+ Hits         14824    15769     +945     
+ Misses        2703     1758     -945     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ting-hong-shieh
ting-hong-shieh marked this pull request as draft August 16, 2026 19:10
@ting-hong-shieh
ting-hong-shieh marked this pull request as ready for review August 18, 2026 15:10
@Gui-FernandesBR
Gui-FernandesBR requested a lite review from Copilot August 19, 2026 02:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aligns the monte-carlo optional dependency set between pyproject.toml and the documented requirements-file install path, so local installs using the Makefile no longer silently skip contextily-dependent Monte Carlo plotting tests and diverge from CI coverage.

Changes:

  • Add contextily>=1.0.0; python_version < '3.14' to requirements-optional.txt to match the monte-carlo extra.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread requirements-optional.txt
multiprocess>=0.70
statsmodels
prettytable
contextily>=1.0.0; python_version < '3.14'

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed, and this is what 0e87657a addresses.

Reproduced on develop with contextily installed and outbound sockets blocked:

6 failed, 12 passed in 0.90s

FAILED test_ellipses_background_types_display_successfully[satellite]
FAILED test_ellipses_background_types_display_successfully[street]
FAILED test_ellipses_background_types_display_successfully[terrain]
FAILED test_ellipses_background_types_display_successfully[CartoDB.Positron]
FAILED test_ellipses_background_works_with_custom_limits
FAILED test_ellipses_background_saves_file_successfully

All six come out of the contextily.bounds2img call at monte_carlo_plots.py:166,
surfaced through the generic handler at :198.

Same blocked-socket run on this branch, with the autouse mock_background_tiles
fixture in place:

18 passed in 0.38s

The fixture patches contextily.bounds2img to return a 2x2 zero-valued tile plus the
Web Mercator extent for the requested bounds, so provider resolution, extent handling
and the plotting path are all still exercised — only the tile server is gone. Wall
clock for the file went from 37s to under half a second.

Verified on Python 3.12.3, contextily 1.7.1. tests/unit/simulation is 191 passed,
4 skipped; pylint on the changed file is 10.00/10.

@ting-hong-shieh
ting-hong-shieh marked this pull request as draft August 19, 2026 17:48
@ting-hong-shieh
ting-hong-shieh marked this pull request as ready for review August 27, 2026 02:11
@ting-hong-shieh

Copy link
Copy Markdown
Author

@Gui-FernandesBR heads-up before you merge: your approval landed at 02:37 on the 19th,
two minutes before the Copilot review, and the test file changed after both. 0e87657a
adds an autouse fixture that mocks contextily.bounds2img, which is what stops the file
from reaching real tile servers once it is no longer skipped. Details and the
blocked-socket comparison are in the review thread.

Worth a second look at that commit specifically. The requirements line is unchanged from
what you approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants